libostree: Variant-related leak plugs and fixes

This tries to avoid leaking GVariantBuilders and GVariants in some
situations. The leaks were usually happening when some error occurred
or because of unclear variant ownership situation.

The former is mostly about making sure that g_variant_builder_clear is
called on builders that didn't finish their variant building process.

The latter is surely more work - sometimes the result of
g_variant_builder_end() should not be passed directly to a function,
but rather stored in a g_autoptr(GVariant), sunk and then passed to a
function. IMO, with an advent of g_autoptr, GVariants should be always
sunk instead of relying on some receiver function sinking it. This
would make an easy-to-follow policy of always sinking your
variants. Functions could then assume that the passed variant is
already sunk. These leaks are still happenning in commands, but they
are less harmful, since that code will not be used by some daemon as a
library routine.

Closes: #291
Approved by: cgwalters

diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c
index 170b7b873b7b2b75d71e20c2492ae7b2803f48b9..f384624c03cc0a049f076db516ace309be72320f 100644 (file)
--- a/src/libostree/ostree-repo-commit.c
+++ b/src/libostree/ostree-repo-commit.c
@@ -2384,7 +2384,7 @@ get_modified_xattrs (OstreeRepo                       *self,
 
       if (label)
         {
-          GVariantBuilder *builder;
+          g_autoptr(GVariantBuilder) builder = NULL;
 
           /* ret_xattrs may be NULL */
           builder = ot_util_variant_builder_from_variant (ret_xattrs,

diff --git a/src/libostree/ostree-repo-libarchive.c b/src/libostree/ostree-repo-libarchive.c
index 57da41d4450fcb63e49a2d20996387c49e71eafe..0d62124dc61558f486e0ae6b9f3c054c5d8ee92e 100644 (file)
--- a/src/libostree/ostree-repo-libarchive.c
+++ b/src/libostree/ostree-repo-libarchive.c
@@ -331,6 +331,7 @@ aic_ensure_parent_dir_with_file_info (OstreeRepoArchiveImportContext *ctx,
 {
   const char *name = glnx_basename (fullpath);
   g_auto(GVariantBuilder) xattrs_builder;
+  g_autoptr(GVariant) xattrs = NULL;
 
   /* is this the root directory itself? transform into empty string */
   if (name[0] == '/' && name[1] == '\0')
@@ -343,8 +344,9 @@ aic_ensure_parent_dir_with_file_info (OstreeRepoArchiveImportContext *ctx,
                             DEFAULT_DIRMODE, cancellable, error))
       return FALSE;
 
+  xattrs = g_variant_ref_sink (g_variant_builder_end (&xattrs_builder));
   return mtree_ensure_dir_with_meta (ctx->repo, parent, name, file_info,
-                                     g_variant_builder_end (&xattrs_builder),
+                                     xattrs,
                                      FALSE /* error_if_exist */, out_dir,
                                      cancellable, error);
 }

diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index 708df5b65b0a13336d8a959af817f9fa863008a7..619657c72b7558fd9d56f205a60736b8659aad1d 100644 (file)
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -1759,6 +1759,8 @@ ostree_repo_pull_one_dir (OstreeRepo               *self,
                           GError                  **error)
 {
   GVariantBuilder builder;
+  g_autoptr(GVariant) options = NULL;
+
   g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}"));
 
   if (dir_to_pull)
@@ -1770,7 +1772,8 @@ ostree_repo_pull_one_dir (OstreeRepo               *self,
     g_variant_builder_add (&builder, "{s@v}", "refs",
                            g_variant_new_variant (g_variant_new_strv ((const char *const*) refs_to_fetch, -1)));
 
-  return ostree_repo_pull_with_options (self, remote_name, g_variant_builder_end (&builder),
+  options = g_variant_ref_sink (g_variant_builder_end (&builder));
+  return ostree_repo_pull_with_options (self, remote_name, options,
                                         progress, cancellable, error);
 }
 

diff --git a/src/libostree/ostree-repo-static-delta-compilation.c b/src/libostree/ostree-repo-static-delta-compilation.c
index 2a28e7db2fdb8a0aeec5ac50f81dc221216c868f..218bbcc84bdea0442e010b8732e1c6e9fdad9ed9 100644 (file)
--- a/src/libostree/ostree-repo-static-delta-compilation.c
+++ b/src/libostree/ostree-repo-static-delta-compilation.c
@@ -1253,7 +1253,7 @@ ostree_repo_static_delta_generate (OstreeRepo                   *self,
   guint min_fallback_size;
   guint max_bsdiff_size;
   guint max_chunk_size;
-  GVariantBuilder metadata_builder;
+  g_auto(GVariantBuilder) metadata_builder = {0,};
   DeltaOpts delta_opts = DELTAOPT_FLAG_NONE;
   guint64 total_compressed_size = 0;
   guint64 total_uncompressed_size = 0;
@@ -1384,16 +1384,18 @@ ostree_repo_static_delta_generate (OstreeRepo                   *self,
       g_autoptr(GVariant) delta_part_content = NULL;
       g_autoptr(GVariant) delta_part = NULL;
       g_autoptr(GVariant) delta_part_header = NULL;
-      GVariantBuilder *mode_builder = g_variant_builder_new (G_VARIANT_TYPE ("a(uuu)"));
-      GVariantBuilder *xattr_builder = g_variant_builder_new (G_VARIANT_TYPE ("aa(ayay)"));
+      g_auto(GVariantBuilder) mode_builder = {0,};
+      g_auto(GVariantBuilder) xattr_builder = {0,};
       guint8 compression_type_char;
 
+      g_variant_builder_init (&mode_builder, G_VARIANT_TYPE ("a(uuu)"));
+      g_variant_builder_init (&xattr_builder, G_VARIANT_TYPE ("aa(ayay)"));
       { guint j;
         for (j = 0; j < part_builder->modes->len; j++)
-          g_variant_builder_add_value (mode_builder, part_builder->modes->pdata[j]);
+          g_variant_builder_add_value (&mode_builder, part_builder->modes->pdata[j]);
         
         for (j = 0; j < part_builder->xattrs->len; j++)
-          g_variant_builder_add_value (xattr_builder, part_builder->xattrs->pdata[j]);
+          g_variant_builder_add_value (&xattr_builder, part_builder->xattrs->pdata[j]);
       }
         
       payload_b = g_string_free_to_bytes (part_builder->payload);
@@ -1403,7 +1405,7 @@ ostree_repo_static_delta_generate (OstreeRepo                   *self,
       part_builder->operations = NULL;
       /* FIXME - avoid duplicating memory here */
       delta_part_content = g_variant_new ("(a(uuu)aa(ayay)@ay@ay)",
-                                          mode_builder, xattr_builder,
+                                          &mode_builder, &xattr_builder,
                                           ot_gvariant_new_ay_bytes (payload_b),
                                           ot_gvariant_new_ay_bytes (operations_b));
       g_variant_ref_sink (delta_part_content);

diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
index 42813e9c322aa524697c9730e3a90f3d9f663ee8..91f9615e042137db36ff73734d050e2244184ade 100644 (file)
--- a/src/libostree/ostree-repo.c
+++ b/src/libostree/ostree-repo.c
@@ -3510,13 +3510,16 @@ ostree_repo_delete_object (OstreeRepo           *self,
 
       if (tombstone_commits)
         {
-          g_autoptr(GVariantBuilder) builder = NULL;
-          builder = g_variant_builder_new (G_VARIANT_TYPE ("a{sv}"));
-          g_variant_builder_add (builder, "{sv}", "commit", g_variant_new_bytestring (sha256));
+          g_auto(GVariantBuilder) builder = {0,};
+          g_autoptr(GVariant) variant = NULL;
+
+          g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}"));
+          g_variant_builder_add (&builder, "{sv}", "commit", g_variant_new_bytestring (sha256));
+          variant = g_variant_ref_sink (g_variant_builder_end (&builder));
           if (!ostree_repo_write_metadata_trusted (self,
                                                    OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT,
                                                    sha256,
-                                                   g_variant_builder_end (builder),
+                                                   variant,
                                                    cancellable,
                                                    error))
             goto out;
@@ -4286,7 +4289,6 @@ ostree_repo_append_gpg_signature (OstreeRepo     *self,
   gboolean ret = FALSE;
   g_autoptr(GVariant) metadata = NULL;
   g_autoptr(GVariant) new_metadata = NULL;
-  g_autoptr(GVariantBuilder) builder = NULL;
 
   if (!ostree_repo_read_commit_detached_metadata (self,
                                                   commit_checksum,
@@ -4954,7 +4956,7 @@ ostree_repo_regenerate_summary (OstreeRepo     *self,
   g_autoptr(GVariant) summary = NULL;
   GList *ordered_keys = NULL;
   GList *iter = NULL;
-  GVariantDict additional_metadata_builder;
+  g_auto(GVariantDict) additional_metadata_builder = {0,};
 
   if (!ostree_repo_list_refs (self, NULL, &refs, cancellable, error))
     goto out;
@@ -4987,7 +4989,7 @@ ostree_repo_regenerate_summary (OstreeRepo     *self,
   {
     guint i;
     g_autoptr(GPtrArray) delta_names = NULL;
-    GVariantDict deltas_builder;
+    g_auto(GVariantDict) deltas_builder = {0,};
     g_autoptr(GVariant) deltas = NULL;
 
     if (!ostree_repo_list_static_delta_names (self, &delta_names, cancellable, error))